-
-
Notifications
You must be signed in to change notification settings - Fork 41
fix: Keep hybrid object data #255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 25
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| try { | ||
| const result = HybridNitroSQLite.execute(dbName, query, params) | ||
| return buildJsQueryResult<Row>(result) | ||
| return result as QueryResult<Row> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rows item contract changed unexpectedly
High Severity
Returning HybridNitroSQLite.execute*() directly exposes native rows behavior instead of the normalized JS shape from buildJsQueryResult. The rows.item callback now comes from native NitroSQLiteQueryResultRows and resolves asynchronously, so callers expecting synchronous rows.item(idx) can get a Promise instead of a row.
Additional Locations (1)
| "Bash(grep:*)" | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local Claude settings committed
Low Severity
The PR adds .claude/settings.local.json, a machine-local configuration file unrelated to the SQLite feature change. Committing local permissions.allow entries (Bash(find:*), Bash(grep:*)) can introduce environment-specific behavior and accidental policy drift in the repository.
| if (index >= rows.size()) { | ||
| return std::nullopt; | ||
| } | ||
| return rows[index]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Row index coercion changes item behavior
Medium Severity
rows.item now casts idx with static_cast<size_t>(idx), which coerces non-integer numbers to integer indices. The previous JS behavior (data[idx]) returned undefined for non-integer keys. This can return the wrong row for values like decimals or NaN, changing query result access semantics.
| export type QueryResult<Row extends QueryResultRow = QueryResultRow> = | ||
| NitroSQLiteQueryResult & { | ||
| readonly rowsAffected: number | ||
| readonly insertId?: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic QueryResult typing lost for results
Low Severity
QueryResult<Row> no longer redefines results as Row[], so the generic type parameter is effectively ignored for main row data. execute<Row>() now returns broadly typed Record<string, SQLiteValue>[], reducing type safety and making typed query APIs less reliable for consumers.


Make sure that the data from Nitro hybrid objects is directly passed to the user-facing JS API.
Note
Medium Risk
Changes the native-to-JS return path for
execute/executeAsyncby returning a sharedHybridNitroSQLiteQueryResultinstead of a copied JS-built object, which could affect result shape/lifetime and batch row counts.Overview
Passes Nitro hybrid query results through to JS without rebuilding them. Native
sqliteExecute()now returns aHybridNitroSQLiteQueryResultdirectly (removingSQLiteExecuteQueryResult), andHybridNitroSQLite::execute()just forwards that shared hybrid object.HybridNitroSQLiteQueryResultnow storesinsertId,rowsAffected,results,rows, andmetadataas its own fields and constructs therowsaccessor up front (including an empty-rows case). JSexecute/executeAsyncstop constructing a new result object and instead return the hybrid object as-is, and TypeScript types are simplified to rely on the Nitro spec (only specializingrowsgenerics).Written by Cursor Bugbot for commit 078f30d. This will update automatically on new commits. Configure here.